Conversation
Files are still concatted, but they are now converted to something the browser understands via `rollup`.
These are now executed in strict context.
Tests need the variant files, which are (still) copied during the build.
robx
left a comment
robx
left a comment
There was a problem hiding this comment.
Mostly a nitpicky review with whatever came to mind while first reading over it. Feel free to ignore parts that don't make sense. Also I still need to have a bit of a look at rollup.
package.json
Outdated
| }, | ||
| "scripts": { | ||
| "build": "eslint --cache --quiet src src-ui && ./git-hash.sh && grunt default", | ||
| "build": "./git-hash.sh && grunt default", |
There was a problem hiding this comment.
the git-hash part here seems like it should be redundant with the new Makefile target
There was a problem hiding this comment.
could we get rid of this script entirely and call npx grunt default from the Makefile instead?
There was a problem hiding this comment.
p.html and the examples and such are still copied with Grunt. I thought is was a bit too much to also change that in this PR. But we could.
There was a problem hiding this comment.
I didn't mean to get rid of Grunt, I meant to get rid of the "build" script in package.json, and to call Grunt directly from the Makefile.
package.json
Outdated
| "scripts": { | ||
| "build": "eslint --cache --quiet src src-ui && ./git-hash.sh && grunt default", | ||
| "build": "./git-hash.sh && grunt default", | ||
| "release": "npm run clean && eslint --cache --quiet src && grunt release", |
There was a problem hiding this comment.
It's referenced in Gruntfile:
var PRODUCTION = (grunt.cli.tasks.indexOf('release') >= 0);
But the only difference seems to be that if PRODUCTION is true it copies over .map files, which is fine anyway?
There was a problem hiding this comment.
It's at least fine in the sense that I'm currently deploying those anyway... https://puzz.link/js/list.js.map
|
@robx thanks for the comments, this should be it. |
|
Having a bit of trouble working with this. Specifically, Any clue? Also, we might want to keep Grunt for concatenation, since that's what gives us source maps and thus decent stack traces on test failure. (This works, when run via |
This ended up trying to set the error flag on the frozen null cell object.
|
I've pushed two fixes to master that should obviate the need for those |
This reverts commit f296cba.
2 participants
This is the minimal version of #178
The build process is still to concat the JS files (as
./dist/js/pzpr.concat.js), but this file is now a ES6 module. This concated file is then transformed for the browser viarollup. The actual code of the module didn't change. The tests do use the modernimport ... from ...syntax to load data.